Add key type tests #35

aljones15 · 2024-07-05T21:08:10Z

Refactors the test suite so a common set of assertions runs on all the curves and not just one of them.

Addresses: #34

dlongley · 2024-07-05T21:26:23Z

test/mock-data.js

@@ -9,22 +9,55 @@ export const mockKey = {
 };

 export const mockKeyEcdsaSecp256 = {
-  type: 'EcdsaSecp256r1VerificationKey2019',
+  type: 'Multikey',


This one needs to be reverted back to EcdsaSecp256r1VerificationKey2019. This is for backwards compatibility tests. The others should be Multikey as they are now.

Reverted here: 28c76f3 but that breaks the id should be set test so will need to further revise this PR.

@dlongley ok so inside of the multkeys map in mock-data I now set the type to Multikey so the P-256 one will pass the should set id test: https://github.com/digitalbazaar/ecdsa-multikey/pull/35/files/28c76f394ed4919f39581d2fdc60611810a2dd92..09a57e00aeebfdc1d25a6e6eee4c64273e64a5ff

Should we test both the backwards compatible EcdsaSecp256r1VerificationKey2019 P-256 and the Multikey P-256? if so I'm assuming for EcdsaSecp256r1VerificationKey2019 the id is not set by default?

This is the test that is failing

ecdsa-multikey/test/assertions.js

Lines 173 to 178 in 09a57e0

it('should auto-set key.id based on controller', async () => {

const {publicKeyMultibase} = serializedKeyPair;

const keyPair = await EcdsaMultikey.from(serializedKeyPair);

_ensurePublicKeyEncoding({keyPair, keyType, publicKeyMultibase});

expect(keyPair.id).to.equal(id);

});

for EcdsaSecp256r1VerificationKey2019

@aljones15, we shouldn't hack the key type to be Multikey just to pass this test. Either the library is supposed to auto-generate an id no matter what the key type was, or it shouldn't do it for a key type of EcdsaSecp256r1VerificationKey2019. For now, I'd say to skip this test if the key type isn't Multikey and file an issue to possibly add id auto-generation for other key types.

@dlongley not sure I hacked the key type to be Multikey as by default generate({curve: 'P-256'}) produces a key with type Multikey. All the test suite does is ensure that all keys in the multikeys mock-data map have type Multikey. The original EcdsaSecp256r1VerificationKey2019 is not mutated and still exists in the suite. The question is should we test both P-256 Multikey and P-256 EcdsaSecp256r1VerificationKey2019, the answer to that appears to be file an issue about it for now as EcdsaSecp256r1VerificationKey2019 does not pass all the tests.

issue created here: #36 for EcdsaSecp256r1VerificationKey2019 in test suite loop and and key id generation.

All we want with the EcdsaSecp256r1VerificationKey2019 key pair is to demonstrate that we can import and use it like any other keypair. It exists so we can run tests against legacy key pairs that use that type. So as long as this PR includes some tests against something that imports using that key type, we're good. That doesn't have to include id generation at this time.

@dlongley the original key type import tests were left as is:

ecdsa-multikey/test/EcdsaMultikey.spec.js

Lines 66 to 77 in 09a57e0

describe('Backwards compat with EcdsaSecp256r1VerificationKey2019', () => {

it('Multikey should import properly', async () => {

const keyPair = await EcdsaMultikey.from(mockKeyEcdsaSecp256);

const data = (new TextEncoder()).encode('test data goes here');

const signature = await keyPair.signer().sign({data});

expect(

await keyPair.verifier()

.verify({data, signature})

).to.be.true;

});

});

Ok, sounds good.

test/mock-data.js

dlongley

I'm not sure about this style, but I don't have a better suggestion for reuse that is as succinct, so I'm going to approve and merge this. It's hard to tell that everything was preserved so I'm just assuming it is -- and maybe some codecov printout will tell us that things still get the same or better coverage somewhere.

dlongley · 2024-07-08T23:22:02Z

Thanks! Going to merge this now.

aljones15 added 10 commits July 5, 2024 15:31

Add keyTypes & make sign and verify configurable.

504cfb1

Assert on id & keyType in sign & verify.

61f812a

Add P-384 & P-521 mock keys.

533c8fb

Add assertions lib & test multiple keyTypes.

51db63f

Add multikey spec & remove sign verify spec.

f044e63

Prefer type Multikey.

8871e5b

Move ensurePublicKeyEncoding to assertions & use keyType.

8882a67

Move round trip-loaded exports to multikey spec.

7443bc2

Move fromJWK and Raw tests to multikey.

9ca8a66

Lint add missing semicolon to assertion.

1e2dbce

aljones15 self-assigned this Jul 5, 2024

aljones15 requested review from kezike, BigBlueHat and dlongley July 5, 2024 21:08

dlongley requested changes Jul 5, 2024

View reviewed changes

aljones15 commented Jul 7, 2024

View reviewed changes

test/mock-data.js Outdated Show resolved Hide resolved

aljones15 added 2 commits July 6, 2024 21:08

Update test/mock-data.js revert to EcdsaSecp256r1VerificationKey2019.

28c76f3

In multikeys mock data set P-256 to type Multikey.

09a57e0

aljones15 requested a review from dlongley July 7, 2024 13:27

dlongley approved these changes Jul 8, 2024

View reviewed changes

dlongley merged commit e9b2c6e into main Jul 8, 2024
5 checks passed

dlongley deleted the add-key-type-tests branch July 8, 2024 23:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add key type tests #35

Add key type tests #35

aljones15 commented Jul 5, 2024 •

edited

Loading

dlongley Jul 5, 2024

aljones15 Jul 7, 2024

aljones15 Jul 7, 2024

dlongley Jul 7, 2024

aljones15 Jul 7, 2024

aljones15 Jul 7, 2024 •

edited

Loading

dlongley Jul 8, 2024

aljones15 Jul 8, 2024

dlongley Jul 8, 2024

dlongley left a comment

dlongley commented Jul 8, 2024

	it('should auto-set key.id based on controller', async () => {
	const {publicKeyMultibase} = serializedKeyPair;
	const keyPair = await EcdsaMultikey.from(serializedKeyPair);
	_ensurePublicKeyEncoding({keyPair, keyType, publicKeyMultibase});
	expect(keyPair.id).to.equal(id);
	});

	describe('Backwards compat with EcdsaSecp256r1VerificationKey2019', () => {
	it('Multikey should import properly', async () => {
	const keyPair = await EcdsaMultikey.from(mockKeyEcdsaSecp256);
	const data = (new TextEncoder()).encode('test data goes here');
	const signature = await keyPair.signer().sign({data});

	expect(
	await keyPair.verifier()
	.verify({data, signature})
	).to.be.true;
	});
	});

Add key type tests #35

Add key type tests #35

Conversation

aljones15 commented Jul 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aljones15 Jul 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dlongley left a comment

Choose a reason for hiding this comment

dlongley commented Jul 8, 2024

aljones15 commented Jul 5, 2024 •

edited

Loading

aljones15 Jul 7, 2024 •

edited

Loading